-
-
Notifications
You must be signed in to change notification settings - Fork 183
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Require Infura project ID #1276
Conversation
613a0f0
to
3e75061
Compare
The network controller now requires an Infura project ID to be provided upon construction. This brings the network controller more in-line with the extension. Runtime validation has been added for the sake of making the merge with the extension network controller easier. Closes #1201
3e75061
to
5cf5f60
Compare
Is it possible to refrain from doing this and take it in the other direction? The goal should be that everything degrades gracefully without Infura connectivity or credentials as a first step, and long-term to make the codebase provider-agnostic and remove hardcoded references to specific providers on this level?. |
This constructor argument won't make a difference either way in terms of how baked-in the Infura networks are. This isn't meant to cement them further, just to improve validation and help us merge the two network controllers. I'd like to remove the special handling of Infura providers as well but... that's not trivial (API changes, configuration, middleware differences, etc.). It'll be easier to tackle that when we have one network controller rather than two. |
@@ -173,7 +173,7 @@ export type NetworkControllerMessenger = RestrictedControllerMessenger< | |||
export type NetworkControllerOptions = { | |||
messenger: NetworkControllerMessenger; | |||
trackMetaMetricsEvent: () => void; | |||
infuraProjectId?: string; | |||
infuraProjectId: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's easy to get the impression that this does cement the provider.. Here or another place good to add a // TODO:
or // FIXME:
on that the intention is that the controller will be made provider-agnostic?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I should elaborate; the path to removing these would be to enhance the capabilities of our custom RPC configuration, such that it can be used to support our Infura networks as well.
We've already started down this path; the localhost e2e test network used to be baked-in, but now we use it as a custom network instead. And when we recently added Linea as a default network, it was added as a custom network.
That is, I'd expect us to delete the built-in networks completely rather than make them optional. Making them optional isn't need not be a step along the path to removing them.
It's long past time to track this work in an issue; I'll set a reminder to create one tomorrow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Issue created: #1279
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
The network controller now requires an Infura project ID to be provided upon construction. This brings the network controller more in-line with the extension. Runtime validation has been added for the sake of making the merge with the extension network controller easier. Closes #1201
The network controller now requires an Infura project ID to be provided upon construction. This brings the network controller more in-line with the extension. Runtime validation has been added for the sake of making the merge with the extension network controller easier. Closes #1201
Description
The network controller now requires an Infura project ID to be provided upon construction. This brings the network controller more in-line with the extension. Runtime validation has been added for the sake of making the merge with the extension network controller easier.
Changes
@metamask/network-controller
infuraProjectId
constructor parameter is now requiredReferences
Closes #1201
Checklist